Skip to content

Conversation

@kotfu
Copy link
Member

@kotfu kotfu commented Jul 21, 2018

Closes #477.

@kotfu kotfu requested a review from tleonhardt as a code owner July 21, 2018 18:57
@codecov
Copy link

codecov bot commented Jul 21, 2018

Codecov Report

Merging #478 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #478   +/-   ##
=======================================
  Coverage   90.96%   90.96%           
=======================================
  Files          11       11           
  Lines        2868     2868           
=======================================
  Hits         2609     2609           
  Misses        259      259

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3f737b4...58e2d8b. Read the comment docs.

tleonhardt
tleonhardt previously approved these changes Jul 21, 2018
Copy link
Member

@tleonhardt tleonhardt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider my couple comments, but everything looks good.

last = first + 10

for x in range(first, last):
self.poutput(x)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Recommend converting x from an int to a str first since poutput is intended to receive a str (even though this will work):

self.poutput(str(x))

def abbrev_hook(self, data: cmd2.plugin.PostparsingData) -> cmd2.plugin.PostparsingData:
"""Accept unique abbreviated commands"""
target = 'do_' + data.statement.command
if not target in dir(self):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Recommend testing for membership using:

if target not in dir(self):

super().__init__(*args, **kwargs)

# register three hooks
self.register_postparsing_hook(self.add_whitespace_hook)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting use case for multiple postparsing hooks. I like the example, thanks for creating it.

last = first + 10

for x in range(first, last):
self.poutput(str(x))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything looks good. TravisCI seems to be having some issues today. I restored one of the jobs since it failed for no good reason. This should be ready to merge once CI completes.

@tleonhardt tleonhardt merged commit fdbc73b into master Jul 21, 2018
@tleonhardt tleonhardt deleted the hook_example branch July 21, 2018 22:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants